-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ADLS #6071
ADLS #6071
Conversation
…-java into storage/ADLSdev
…-java into storage/ADLSdev
…-java into storage/ADLSdev
leaseaccessconditions on method to string leaseId
accessConditions, permissions, umask, context)); | ||
} catch (RuntimeException ex) { | ||
return monoError(logger, ex); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can see, the only difference between this and the FileClient create is the pathResourceType. I think we could move all the code for these methods into path client if we add a private final PathResourceType field that gets initialized to the right type in the constructors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...ile-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeDirectoryAsyncClient.java
Outdated
Show resolved
Hide resolved
* | ||
* @return Information about the created directory. | ||
*/ | ||
public PathInfo create() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as async about moving into PathClient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* @return the updated PathClientBuilder object | ||
* @throws IllegalArgumentException If {@code endpoint} is {@code null} or is a malformed URL. | ||
*/ | ||
public PathClientBuilder endpoint(String endpoint) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should support IP style endpoints
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
metadata. | ||
*/ | ||
sb.append(entry.getKey()).append('=') | ||
.append(new String(Base64.getEncoder().encode(entry.getValue().getBytes(Charset.forName("UTF-8"))), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arset.forName("UTF-8") [](start = 94, length = 22)
This is my bad, but I think there's a StandardCharsets.UTF-8 that would be preferable to a string literal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
...ure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/PathAsyncClient.java
Outdated
Show resolved
Hide resolved
* @param accessControl {@link PathAccessControl} | ||
* @return A reactive response containing the resource info. | ||
*/ | ||
public Mono<PathInfo> setAccessControl(PathAccessControl accessControl) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is splitting this api into permissions and acl after preview?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public DataLakeDirectoryAsyncClient getDirectoryAsyncClient(String directoryName) { | ||
if (ImplUtils.isNullOrEmpty(directoryName)) { | ||
throw logger.logExceptionAsError(new IllegalArgumentException("'directoryName' can not be set to null")); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Objects.requireNonNull can make this more concise?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blobs does it this way, but issue created here #6106
/** | ||
* Represents the components that make up an Azure Storage SAS' query parameters. This type is not constructed directly | ||
* by the user; it is only generated by the {@link DataLakeServiceSasSignatureValues} type. Once generated, it can be | ||
* set on a {@link PathClientBuilder} object to be constructed as part of a URL or it can be encoded into a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the builder method actually accepts a string, not SasQueryParameters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. We will need to take note to change this everywhere else as well
...ile-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeDirectoryAsyncClient.java
Show resolved
Hide resolved
/azp run java - storage - ci |
Azure Pipelines successfully started running 1 pipeline(s). |
DataLakeFileClient fileClient = fileSystemClient.getFileClient("HelloWorld.txt"); | ||
|
||
String data = "Hello world!"; | ||
InputStream dataStream = new ByteArrayInputStream(data.getBytes(StandardCharsets.UTF_8)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add in creating a directory client and putting a file under it? That way we show all the basic client types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding an issue for support for url encoding #6130 |
Resolves #4516